-
Notifications
You must be signed in to change notification settings - Fork 350
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix sub-component references in react-console #894
Fix sub-component references in react-console #894
Conversation
import { MenuItem } from 'patternfly-react'; | ||
|
||
// FIXME: DropdownButton is inaccessible after 1f3411a6e708e833e48921380ae736cf5f79862b commit | ||
import DropdownButton from 'patternfly-react/dist/esm/components/Button/DropdownButton'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jeff-phillips-18 , any idea?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mareklibra I will look into this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah... so this was broken for the patternfly-react packages by #853 like you say.
Unfortunately, I can think of only 2 fixes:
-
Create directories for all subcomponents and move them there. This would, however, be a breaking change externally as anyone doing path imports would no longer find the subcomponent in the same location.
-
Add subcomponents to the main component and reference them through that (only required in patternfly-react packages not externally). That is, Button would have Button.Dropdown and you would import Button and use that . Similar to Row and Col in Grid.
Can you think of any other options?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create directories for all subcomponents and move them there.
I would avoid that as well.
That is, Button would have Button.Dropdown and you would import Button and use that . Similar to Row and Col in Grid.
This is probably the easiest fix if the encapsulation by #853 is really needed (I don't see much benefit of that).
It makes sense in case of Grid as Col and Row are real subcomponents.
But Button and Dropdown are equal in the hierarchy (just different specialisation).
So doing it properly, we should introduce Buttons
and reference via that, including the recent Button
.
Are there any other occurrences except buttons? If not or not many, I would not care and add Button.Dropdown
. Otherwise, a systematical approach should be preferred.
Can you think of any other options?
Just more complicated ones (none preferred by me):
- introduce reference-only subcomponents handling the paths. We would get the liberty to hand-pick subcomponents for export.
- modify build to export/generate components based on re-exporting from the
index.js
files
packages/patternfly-3/react-console/src/SerialConsole/SerialConsoleActions.js
Outdated
Show resolved
Hide resolved
c02b45f
to
0db0cc2
Compare
Pull Request Test Coverage Report for Build 3257
💛 - Coveralls |
79eab76
to
2425fc6
Compare
@jeff-phillips-18 , @priley86 , any idea how to import components like Former
is newly babelized to
which is missing. |
2425fc6
to
3dfe56d
Compare
This will be rebased once #916 lands. |
3dfe56d
to
d76bcfb
Compare
Follow-up for 1f3411a. Sub-components (like Grid.Col) are no more exported individually.
d76bcfb
to
4fc5d38
Compare
Rebased after #916 merge. |
PatternFly-React preview: https://894-pr-patternfly-react-patternfly.surge.sh |
Follow-up for 1f3411a , resp #853.
Sub-components (like Grid.Col) are no more exported individually.